Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to README to reflect Workbox usage. #5111

Merged
merged 3 commits into from
Oct 1, 2018

Conversation

jeffposnick
Copy link
Contributor

R: @gaearon @Timer

This is a continuation of the fix for #3864, with additional changes to reflect the usage of Workbox and the fact that setting proper Cache-Control headers on the generated service worker file should (hopefully!) no longer be a concern for modern web browsers.

This PR depends on #4169 being merged.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@@ -1833,17 +1832,6 @@ following into account:
instructions for using other methods. _Be sure to always use an
incognito window to avoid complications with your browser cache._

1. If possible, configure your production environment to serve the generated
Copy link
Contributor Author

@jeffposnick jeffposnick Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per https://developers.google.com/web/updates/2018/06/fresher-sw, I think we can just leave this section out now—I don't know how much the README needs to cater to older browsers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since service workers are only supported in evergreen browsers, I'm fine with this.

@iansu
Copy link
Contributor

iansu commented Sep 26, 2018

@jeffposnick Are you proposing we merge #4169 as is and then apply these changes after that?

[`runtimeCaching`](https://github.com/GoogleChrome/sw-precache#runtimecaching-arrayobject)
option in the `SWPrecacheWebpackPlugin` section of
[`webpack.config.prod.js`](../config/webpack.config.prod.js).
runtime caching strategy for those requests, you can add in a [`runtimeCaching`](https://developers.google.com/web/tools/workbox/modules/workbox-webpack-plugin#generateSW-runtimeCaching) to section to a custom `workbox.config.js` file.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workbox.config.js file is something that was alluded to, but I believe it's just hypothetical right now.

I'm assuming that that functionality is going to be added to the next branch at some point by the c-r-a maintainers, and this reference to workbox.config.js could at that point be linked to whatever documentation is written for that new functionality.

(If I'm getting those assumptions wrong, let me know!)

@jeffposnick
Copy link
Contributor Author

Right, once #4169 is merged and things switch over to Workbox, there are some parts of the README that would need to be updated, and that's included in this PR. (#4169 is not my PR, so I can't just add these changes into that.)

There are also a couple of things that have changed in terms of browser functionality since #3864 was last addressed, and this PR reflects that.

@Timer Timer added this to the 2.0.0 milestone Sep 27, 2018
@gaearon gaearon mentioned this pull request Sep 27, 2018
25 tasks
@Timer
Copy link
Contributor

Timer commented Sep 28, 2018

Can you please remove the references to configuration for now? No one has championed a PR for this yet.

@Timer
Copy link
Contributor

Timer commented Oct 1, 2018

Merging, cla: #3924 (comment)

@Timer Timer changed the base branch from next to master October 1, 2018 00:12
@Timer Timer merged commit 3c70340 into facebook:master Oct 1, 2018
zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Oct 2, 2018
* Updates to README to reflect Workbox usage.

* Update README.md

* Update README.md
@jeffposnick jeffposnick deleted the workbox-readme-updates branch October 8, 2018 21:04
@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants